Conversation
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
| threadId == appNavigationState.navigationState.currentThreadId() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe changing the receiver and parameter is a bit clearer, what do you prefer @jmartinesp ?
/**
* Used to check if a notifiableEvent should be ignored based on the current application navigation state.
*/
private fun AppNavigationState.shouldIgnoreEvent(event: NotifiableEvent): Boolean {
if (!isInForeground) return false
return navigationState.currentSessionId() == event.sessionId &&
when (event) {
is NotifiableRingingCallEvent -> {
// Never ignore ringing call notifications
// Note that NotifiableRingingCallEvent are not handled by DefaultNotificationDrawerManager
false
}
is FallbackNotifiableEvent -> {
// Ignore if the room list is currently displayed
navigationState is NavigationState.Session
}
is InviteNotifiableEvent,
is SimpleNotifiableEvent -> {
event.roomId == navigationState.currentRoomId()
}
is NotifiableMessageEvent -> {
event.roomId == navigationState.currentRoomId() &&
event.threadId == navigationState.currentThreadId()
}
}
}There was a problem hiding this comment.
Yes, it's easier to read like this I'd say.
jmartinesp
left a comment
There was a problem hiding this comment.
It works like a charm, thanks! The proposed changes to shouldIgnoreRegardingApplicationState in the comment above make sense to me since they seem easier to understand than the current implementation.
Also, it seems like there are some issues with the tests to fix before merging.
jmartinesp
left a comment
There was a problem hiding this comment.
Odd, the message above should have been an approval and not just a comment.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #6190 +/- ##
===========================================
+ Coverage 81.42% 81.46% +0.04%
===========================================
Files 2569 2569
Lines 69699 69714 +15
Branches 8941 8943 +2
===========================================
+ Hits 56751 56792 +41
+ Misses 9628 9602 -26
Partials 3320 3320 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|



Content
Add more test to ensure that incoming events are correctly ignored regarding the application state.
Motivation and context
Improve behavior of fallback notification.
Screenshots / GIFs
Tests
Tested devices
Checklist